fix(form): a wizard with allowSkip no longer submits past the fields you skipped - #3030
Merged
Conversation
…s you skipped
`allowSkip` let the user jump to any step from the indicator, and handleStepClick
did so without validating anything on the way. Since a wizard mounts ONE step at
a time and react-hook-form only validates the fields currently MOUNTED, a
required field on a step nobody opened was never registered, never validated, and
simply absent from the payload.
Measured against the unfixed component — 3 steps, required `owner` on step 2,
allowSkip, click step 3's indicator, fill it, hit Create:
createCalls: 1
payload: { subject: 'S1', notes: 'S3' } // `owner` missing entirely
UI mentions "required": false // nothing said so
An invalid create went out and the client said nothing about why — #2959's
validation half, wearing a wizard's clothes.
The final submit now checks the WHOLE declared field set: when something is
outstanding it returns the user to the first step holding one, marks that step's
indicator (data-error, destructive circle + icon), names the fields in a toast,
and sends nothing. Conditional rules are honoured — the check runs on the
canonical resolveFieldRuleState, the same engine the form renderer and the
server's rule-validator use, so a field hidden by visibleWhen or not yet required
by requiredWhen is not demanded. The sequential path is unaffected: a forward jump
is refused without allowSkip, so Next already validated each step on the way.
Also in WizardForm:
- FormView.columns is now honoured (spec key, previously dropped) — grid width is
the view's `columns`, else the step's own. No widest-section fallback, unlike
the tabbed/split hosts: wizard steps never share a viewport, so each keeps its
authored width.
- the root gained `@container`. The step grid is sized with container queries, so
without a container ancestor every @md:/@2XL: variant was inert and a step
declaring 2 columns rendered single-column. Found by running it in a browser —
the class was present all along, which is why asserting the class alone had
missed it; the test now also asserts the container ancestor.
Browser-verified end to end (zero-backend harness): skip -> submit refused, back
on the offending step with only that indicator marked and the two field names in
the toast, no request sent; fill it -> marker clears and the create carries all
three steps' values.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two
WizardFormitems in one PR, as they touch the same file: the verifiedallowSkipvalidation hole, and the view-levelcolumnsgap left out of #3018.1.
allowSkiplet required fields through unvalidatedhandleStepClick(WizardForm.tsx:397) jumps to any step underallowSkipwithout validating anything on the way. A wizard mounts one step at a time, and react-hook-form only validates the fields currently mounted — so a required field on a step nobody opened was never registered, never validated, and simply absent from the payload.Measured against the unfixed component (3 steps, required
owneron step 2,allowSkip: true, click step 3's indicator, fill it, hit Create):An invalid create went out and the client said nothing about why — #2959's validation half, wearing a wizard's clothes. This was the hypothesis I flagged as unverified two PRs ago; it reproduces.
Fix. The final submit checks the whole declared field set. When something is outstanding it returns the user to the first step holding one, marks that step's indicator (
data-error="true", destructive circle + icon), names the fields in a toast, and sends nothing.Conditional rules are honoured: the check runs on the canonical
resolveFieldRuleStatefrom@object-ui/core— the same engine the form renderer and the server's rule-validator use — so a field hidden byvisibleWhenor not yet required byrequiredWhenis not demanded. No second dialect.The sequential path is unaffected: a forward jump is refused without
allowSkip(verified:forwardJumpRefused: true,indicatorDisabled: true), so Next already validated each step on the way.2.
FormView.columnsin the wizardThe spec key was being dropped, same as in Tabbed/Split before #3018. Grid width is now the view's
columns, else the step's own. No widest-section fallback here, unlike the tabbed/split hosts: wizard steps never share a viewport, so there is no shared grid to size and each step keeps its authored width. That difference is documented.A bug the browser caught that the test hadn't
While verifying, a step declaring
columns: 2still rendered single-column. The step grid is sized with container queries, andWizardForm's root carried no@container— so every@md:/@2xl:variant was inert.TabbedFormandSplitFormboth have the wrapper; the wizard never did.My unit test had asserted only that the grid class was present, which it always was. The test now also asserts the grid has an
@containerancestor — the precondition that was actually missing.Tests
packages/plugin-form/src/wizardSkipValidation.test.tsx— 6 cases, run against the unfixed component first: 3 failed, 3 passed, exactly as intended.mainexpected null to be truthy— no bounce, it submittedcolumnsexpected null not to be nullcolumnsThe three failures are the defect proofs; the three passes are regression guards against my own change. 39 files / 300 tests green across
plugin-form+ the form renderer;turbo type-checkgreen; lint 0 errors.Browser verification
Zero-backend harness (deleted before commit),
allowSkip+columns: 2+ two required fields on the middle step:Please complete the required fields: Owner, Team, andwindow.__lastCreate === null— nothing sent;{subject, notes, owner, team}— all three steps' values;columns: 2renders 2-up after the@containerfix.Note: the toast joins field names with
,rather than the、the sibling implementation inform.tsxuses — this is a new English string and rule #-1 wants English punctuation. I did not touchform.tsx.🤖 Generated with Claude Code